Skip to content

fix(drivers/smb): close smb sessions during transfers - #2579

Open
shoyuf wants to merge 5 commits into
OpenListTeam:mainfrom
maybeLab:fix/smb-session-lifecycle
Open

fix(drivers/smb): close smb sessions during transfers#2579
shoyuf wants to merge 5 commits into
OpenListTeam:mainfrom
maybeLab:fix/smb-session-lifecycle

Conversation

@shoyuf

@shoyuf shoyuf commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary / 摘要

Fix SMB session lifecycle handling during long transfers.

  • Store the *smb2.Session in the SMB driver so cleanup can call Session.Logoff(), not only Share.Umount().

  • Add connection leases around SMB operations so idle refresh does not replace a session while a file stream, upload, or same-storage copy is still active.

  • Keep the SMB download lease alive until the returned Link stream is closed.

  • Log off partially initialized sessions if Mount fails after Dial succeeds.

  • This PR has breaking changes.

  • This PR changes public API, config, storage format, or migration behavior.

  • This PR requires corresponding changes in related repositories.

Related repository PRs:

  • OpenList-Frontend:
  • OpenList-Docs:

Related Issues / 关联 Issue

Fixes #2578
Related to #2228

Testing / 测试

  • gofmt drivers/smb/driver.go drivers/smb/util.go
  • GOCACHE=/private/tmp/openlist-go-build-cache GOMODCACHE=/private/tmp/openlist-go-mod-cache GOPROXY=https://goproxy.cn,direct go test ./drivers/smb
  • go test ./...
  • Manual test / 手动测试:

go test ./... was attempted, but unrelated existing packages fail:

  • multiple drivers/offline packages fail on Go vet non-constant format string diagnostics;
  • pkg/aria2/rpc expects a local aria2 RPC server at localhost:6800.

The SMB package itself compiles successfully.

Checklist / 检查清单

  • I have read CONTRIBUTING.
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
  • I have formatted the changed code with gofmt.
  • I have requested review from relevant maintainers or code owners where applicable.

AI Disclosure / AI 使用声明

  • This PR includes AI-assisted content.

Tools used / 使用工具:

  • ChatGPT
  • Codex
  • GitHub Copilot
  • Claude
  • Gemini
  • Other:

Usage scope / 使用范围:

  • Code generation / 代码生成

  • Refactoring / 重构

  • Documentation / 文档

  • Tests / 测试

  • Review assistance / 审查辅助

  • I have reviewed and validated all AI-assisted content included in this PR.

  • I have ensured that all AI-assisted commits include Co-Authored-By attribution.

  • I can reproduce all AI-assisted content included in this PR without any AI tools.

@shoyuf
shoyuf force-pushed the fix/smb-session-lifecycle branch from 117c61c to ce34736 Compare June 4, 2026 07:11
@shoyuf
shoyuf marked this pull request as ready for review June 4, 2026 07:13
xrgzs
xrgzs requested changes Jun 4, 2026
View reviewed changes
Comment thread .github/workflows/test_image.yml Outdated
@shoyuf
shoyuf force-pushed the fix/smb-session-lifecycle branch from fdc6f0a to ce34736 Compare June 4, 2026 12:10
@shoyuf
shoyuf requested a review from xrgzs June 4, 2026 12:12
@xrgzs xrgzs changed the title fix(smb): close smb sessions during transfers fix(drivers/smb): close smb sessions during transfers Jun 17, 2026
@xrgzs xrgzs added the Module: Driver Driver-Related Issue/PR label Jun 17, 2026
elysia-best
elysia-best reviewed Jul 1, 2026
View reviewed changes
Comment thread drivers/smb/util.go Outdated
elysia-best
elysia-best reviewed Jul 1, 2026
View reviewed changes
Comment thread drivers/smb/util.go Outdated
shoyuf and others added 2 commits July 20, 2026 21:15
@shoyuf @codex
refactor(smb): address connection review feedback
250bde5 
- Reuse the existing locked SMB initialization helper.
- Remove the obsolete connection-check wrapper.

Signed-off-by: shoyuf <[email protected]>
Co-authored-by: Codex <[email protected]>
@shoyuf @codex
chore(smb): merge upstream main
d7c630d 
- Merge the latest upstream changes into the SMB lifecycle branch.
- Preserve connection lifecycle state while adopting atomic.Int64 timestamps.

Signed-off-by: shoyuf <[email protected]>
Co-authored-by: Codex <[email protected]>
@shoyuf
shoyuf requested a review from elysia-best July 20, 2026 13:22
@shoyuf @codex
refactor(smb): remove unused init wrapper
ba137cf 
- Remove the unreferenced singleflight initialization wrapper.
- Drop imports used only by the deleted wrapper.

Signed-off-by: shoyuf <[email protected]>
Co-authored-by: Codex <[email protected]>
@jyxjjj

jyxjjj commented Jul 21, 2026

Copy link
Copy Markdown
Member

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jul 21, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba137cf3e0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread drivers/smb/driver.go Outdated
_ = d.fs.Umount()
}
return nil
return d.closeFS()

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep SMB Drop cleanup best-effort

When the SMB server is already unreachable or the TCP session is half-closed, closeFS() can return errors from Umount, Logoff, or conn.Close; DisableStorage and UpdateStorage treat any Drop error as fatal, so this change can prevent admins from disabling or updating a broken SMB mount even though teardown has already cleared the local SMB handles. The previous implementation ignored Umount errors, matching the best-effort close behavior used by similar network drivers, so Drop should avoid propagating these cleanup failures or only log them.

Useful? React with 👍 / 👎.

@shoyuf shoyuf Jul 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 58999a0. SMB.Drop() now treats teardown as best-effort: it still calls closeFS() to clear local handles but ignores cleanup errors so disable and update operations can continue. Verified with go test ./drivers/smb.

@shoyuf @codex
fix(smb): keep drop cleanup best-effort
58999a0 
- Ignore SMB teardown errors after clearing local connection handles.
- Allow storage disable and update operations to continue after cleanup failures.

Signed-off-by: shoyuf <[email protected]>
Co-authored-by: Codex <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments
@xrgzs xrgzs Awaiting requested review from xrgzs
@elysia-best elysia-best Awaiting requested review from elysia-best

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

Module: Driver Driver-Related Issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] SMB sessions leak during long transfers until Windows connection limit is reached

4 participants

@shoyuf @jyxjjj @xrgzs @elysia-best